Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[otbn,dv] Model a glitch on the STATUS register #24569

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rswarbrick
Copy link
Contributor

This fixes a problem problem that was visible in the otbn_escalate test. It is caused by the design bug that is tracked in issue #23903. The one cycle glitch wouldn't be a big deal but we might be really unlucky and read the register at just the wrong time!

The change in sim.py changes otbnsim to correctly model the design's behaviour, with a whopping great TODO message that explains how to undo the change again.

However tracking down whether this fixes everything found that this doesn't completely fix the otbn_escalate test, which still occasionally fails! Delving into the problem carefully, you find that the model's status signal (represented as status_q in otbn_core_model.sv) normally changes at the same time as the register top's status_qs signal. This is reasonably sensible, and means the timing in the scoreboard matches things correctly if we read from STATUS. Unfortunately, the model's signal changes state a cycle earlier, at the same time as status_q, for some status changes (in particular, the changes that you get when you start to lock after an RMA request). This means that there is another window where an unfortunately-timed register read will get the wrong value!

Fixing this needs a bit of an overhaul of the logic in otbnsim. The simplest fix is probably to make it the modelled status signal change earlier and then to register the value and recreate status_qs in otbn_core_model.sv.

@rswarbrick rswarbrick added Component:DV DV issue: testbench, test case, etc. IP:otbn labels Sep 12, 2024
@rswarbrick rswarbrick requested a review from a team as a code owner September 12, 2024 17:01
@rswarbrick rswarbrick requested review from matutem and removed request for a team September 12, 2024 17:01
This fixes a problem problem that was visible in the otbn_escalate
test. It is caused by the design bug that is tracked in issue lowRISC#23903.
The one cycle glitch wouldn't be a big deal but we might be really
unlucky and read the register at just the wrong time!

The change in sim.py changes otbnsim to correctly model the design's
behaviour, with a whopping great TODO message that explains how to
undo the change again.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:DV DV issue: testbench, test case, etc. IP:otbn
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant